Skip to content

[skip changelog] Update client_example with proxy testing #1162

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 2, 2021

Conversation

silvanocerza
Copy link
Contributor

Please check if the PR fulfills these requirements

  • The PR has no duplicates (please search among the Pull Requests
    before creating one)
  • The PR follows our contributing guidelines
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)
  • UPGRADING.md has been updated with a migration guide (for breaking changes)
  • What kind of change does this PR introduce?

Adds some examples to the client_example project used to "test" gRPC comunication.


See how to contribute

@silvanocerza silvanocerza requested a review from rsora February 1, 2021 13:19
# Client example

This a client that simulates a gRPC consumer. We're using this for the time being to test the interaction with the gRPC
interface, hopefully in the future we'll have proper integration tests.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The purpose of client_example is not testing. The purpose is documentation.

Copy link
Contributor

@rsora rsora left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As we discussed internally, the original purpose of the client_example core was to provide code and a bit of documentation on how to use the CLI gRPC interface, but we would like to leverage it in the future as a component of our integration tests suite, this PR is a step in that direction.

@silvanocerza silvanocerza merged commit b3fb43f into master Feb 2, 2021
@silvanocerza silvanocerza deleted the scerza/proxy-tests branch February 2, 2021 10:53
@per1234
Copy link
Contributor

per1234 commented Feb 2, 2021

I really don't understand why there is this trend of trying to use client_example for a purpose it was never intended for. This already happened once last year and it made it much less useful for the sake of documentation (#353). I don't see why we can't just write dedicated integration tests and leave the client example as an example.

I'd like to get input on this from the creator of the example: @cmaglie

@silvanocerza
Copy link
Contributor Author

My main concern with the client_example is that it's practically never run but to verify that the gRPC has not been broken. Also it's not run automatically so it's often broken without anyone noticing.

Ideally well written tests can, and should, act as documentation for developers. I think right now the client_example fails on both since it's not run automatically and it doesn't cover all cases.

My goal is to remove it or adapt it in favour of integration testing, make it run automatically so we know if the gRPC interface has been broken, and also cover more cases.

@per1234
Copy link
Contributor

per1234 commented Feb 2, 2021

Running client_example automatically is utterly trivial. It would be less work to add it to the workflow than it has been to talk about it.

@per1234
Copy link
Contributor

per1234 commented Feb 2, 2021

I'm simply not convinced by the test as documentation argument. When writing tests, the sole goal should be to test. When writing documentation the sole goal should be to document. If you try to do both at the same time, you're going to end up needing to make compromises.

@cmaglie
Copy link
Member

cmaglie commented Feb 2, 2021

I'd like to get input on this from the creator of the example: @cmaglie

Heh, it started as a testing program to try out the grpc interface in the very early times but, once the gRPC API has been bootstrapped, to not waste it we transformed this test program into an example of how to use the grpc interface, something like a minimal client stub to tinker or to develop on.

I'm simply not convinced by the test as documentation argument. When writing tests, the sole goal should be to test. When writing documentation the sole goal should be to document. If you try to do both at the same time, you're going to end up needing to make compromises.

I agree, we have the integration test framework, we should add tests there.

Also, right now we have a single example_client that runs a bunch of random actions, IMHO it would make more sense to split it into smaller examples that show one feature at a time... but I don't know if it will ever happen, I guess this is a very low priority.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants